Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use __attribute__ when building with Clang on Windows #1115

Merged
merged 1 commit into from
Sep 14, 2022
Merged

Use __attribute__ when building with Clang on Windows #1115

merged 1 commit into from
Sep 14, 2022

Conversation

bjosv
Copy link
Contributor

@bjosv bjosv commented Sep 14, 2022

Since Clang supports __attribute__ we can avoid disabling it and use packed sdshdrX structs when building with Clang on Windows. This also make sure we don't affect subsequent header files that require __attribute__.

Note:
Clang attempts to be a drop-in replacement for MSVC and therefor defines _MSC_VER to be able to use existing libraries/headers.

Fixes #1042

@bjosv
Use __attribute__ when building with clang on windows
4ee74a3 
Since clang supports __attribute__ we can avoid disabling
it and use packed sdshdr structs. This also make sure we dont
affect subsequent header files that require __attribute__.

Note:
Clang attempts to be compatible with MSVC and defines _MSC_VER
@bjosv
Copy link
Contributor Author

bjosv commented Sep 14, 2022

As I understand it we don't want to change to much in sds.c/.h, but this is an option to fix #1042

@michael-grunder
Copy link
Collaborator

Clang attempts to be a drop-in replacement for MSVC and therefor defines _MSC_VER

It defines __GNUC__ as well which I recently learned the hard way. 😄

As I understand it we don't want to change to much in sds.c/.h, but this is an option to fix #1042

We may actually have more leeway here because we did a mass renaming of sds symbols to hi_<symbol> when Redis last upgraded the version of hiredis it builds.

I'll get this merged.

@michael-grunder michael-grunder merged commit bd9ccb8 into redis:master Sep 14, 2022
@michael-grunder michael-grunder mentioned this pull request Sep 15, 2022
Closed
@bjosv bjosv deleted the windows-clang branch September 15, 2022 06:05
@bjosv
Copy link
Contributor Author

bjosv commented Sep 15, 2022

It defines __GNUC__ as well which I recently learned the hard way. smile

Ouch :)
It's a bit funny that one can find stackoverflow post like How to tell Clang to stop pretending to be other compilers?

@Danvil
Copy link

Danvil commented Sep 29, 2022

Thank you for fixing the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mmintrin.h: error: cannot initialize a parameter ...
3 participants
@bjosv @michael-grunder @Danvil